Skip to content

GM 4.0 workaround (partially tested)#23

Merged
ilyvion merged 17 commits intoQuestionable-Content-Extensions:developfrom
trlkly:develop
Jan 15, 2018
Merged

GM 4.0 workaround (partially tested)#23
ilyvion merged 17 commits intoQuestionable-Content-Extensions:developfrom
trlkly:develop

Conversation

@trlkly
Copy link
Copy Markdown
Contributor

@trlkly trlkly commented Jan 13, 2018

See #21. This has only been tested on the full code, not the source. Updated node to avoid a travis error.

This shim has been tested on the release, but not on source.
Modified the grant-none shim to not take effect if GM_ functions still exist.
The original grant-none-shim.js would run even on GM versions that include the GM_ functions. This could cause breakage. Fixed to only create new functions if they don't already exist.
@ilyvion
Copy link
Copy Markdown
Contributor

ilyvion commented Jan 13, 2018

Hey, I really appreciate this! If you don't mind, though, I'd much rather have

  1. the shim code in-repo than in a Gist under someone else's control (mostly because I don't want to have to ask my users to trust another repo/source altogether other than this one/me, and because there'd be nothing stopping you from just deleting the Gist under our feet). I see one of your earlier commits had the shim be in-repo, but that it caused my JS-linter to hiccup due to the ES6 code. I really need to update my build environment... for multiple reasons(*).
  2. the inline shim somewhere more sensible than the userscriptBanner variable.

Still, to honor your contribution, I'm most likely going to accept your pull request and then make my modifications afterwards, so you'll be part of the commit history. :)

(*) I've made another userscript that uses a build system based on the one I made here, but with various improvements. Next I get to make edits to this repo, I'll backport them here. Among the improvements are ES6 support, moving the userbanner out into a separate file rather than crammed into a variable and by far the most effort-saving: automatic publishing of new releases through Travis when tagging a commit.

@trlkly
Copy link
Copy Markdown
Contributor Author

trlkly commented Jan 13, 2018

No worries. I always intended this to be a stopgap, not permanent. The idea was to have a quick fix so people could use it in GM 4. I shimmed my own copy and was surprised I could make it work so easily. Hence wanted to share it, so I put it in the repo the easiest way I could just to get it done. That said, I went ahead and tried to fix what you mentioned (as you said you don't have time to work on it right now).

I think I've got it all in repo now. I put all of the shim code into one file, removed the stuff that didn't have to do with storage, and reformatted until I got it to validate. Then I added it to the Grunt file.

Unfortunately, I still can only "partially test" it. I added the script directly to the minified version and checked that it ran on GM 3, GM 4, and the latest TamperMonkey. But I seem to be unable to actually build the script from scratch. I ran vagrant up almost an hour ago, and it's still sitting there, still with no VM running.

I definitely agree that having it built by Travis would be an improvement. I spent way too long trying to find a way to pull out the file it produced when testing.

@ilyvion
Copy link
Copy Markdown
Contributor

ilyvion commented Jan 13, 2018

When you say vagrant up is just sitting there, has it not given ANY output at all? I mean, within seconds of running that command, you should at least start seeing stuff like this when running it for the first time (which I just attempted on this computer, on which I've never had vagrant before):

PS P:\Temp\client> vagrant up
Bringing machine 'default' up with 'virtualbox' provider...
==> default: Box 'hashicorp/precise64' could not be found. Attempting to find and install...
    default: Box Provider: virtualbox
    default: Box Version: >= 0
==> default: Loading metadata for box 'hashicorp/precise64'
    default: URL: https://vagrantcloud.com/hashicorp/precise64
==> default: Adding box 'hashicorp/precise64' (v1.1.0) for provider: virtualbox
    default: Downloading: https://vagrantcloud.com/hashicorp/boxes/precise64/versions/1.1.0/providers/virtualbox.box
    default:
==> default: Successfully added box 'hashicorp/precise64' (v1.1.0) for 'virtualbox'!
==> default: Importing base box 'hashicorp/precise64'...
==> default: Matching MAC address for NAT networking...
==> default: Checking if box 'hashicorp/precise64' is up to date...
[...]

Mine eventually failed running because I've not enabled or turned on VT-x, but I did at least get output and stuff happening almost immediately.

I haven't got time to deal with this pull request today, but I'm most likely going to have time tomorrow, so I'll process your contribution then. :)

@trlkly
Copy link
Copy Markdown
Contributor Author

trlkly commented Jan 15, 2018

Yes, it just sits there after I put in the vagrant up command, with no output. I even tried running it as administrator, to see if that was the problem. But no dice. If it helps, I don't think it's a problem with anything you've done. (And I do have VT-x enabled: I did so a long time ago for faster VMs. I use them a lot, mostly recreationally.)

Anyways, I actually came back to note that there is one downside to my patch, in that it resets your settings on any userscript engine that supports the GM object, since the settings are now stored in a new place. So settings such as displaying all characters and places or having the small ribbon get reset.

I decided that using the namespace as a name was a bad idea if you ever write another script, so I now use the initials from the script name. And since I was already updating anyways, I thought to reverse my previous decision and continue to use GM_ storage if possible, so settings won't be lost unless necessary. 

I also added a shim for GM_info, which I neglected before. It currently exists in GM 4, but it may be removed in the future, hence the weird testing. 

Not sure how many edits I should do in one commit, but this one is tested (as much as the rest).
@ilyvion ilyvion merged commit 1418387 into Questionable-Content-Extensions:develop Jan 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants